SapMachine #2250: Add adjustments to heap dumps for the buildpack#2251
SapMachine #2250: Add adjustments to heap dumps for the buildpack#2251schmelter-sap wants to merge 13 commits into
Conversation
|
Hello @schmelter-sap, this pull request fulfills all formal requirements. |
|
Couple of minor adjustments might be needed in globals.hpp |
|
|
||
| // SAPJVM 2026-05-06: Fill with zeros, if we don't dump the whole content of the array. | ||
| if (fill_with_zero > 0) { | ||
| writer->write_zero((u4) fill_with_zero * type_size); |
There was a problem hiding this comment.
Should this be size_t like it is used in the declaration of write_zero ?
There was a problem hiding this comment.
The length in bytes in this function always uses u4. So for consistency with other code I kept it that way.
There was a problem hiding this comment.
I agree with @MBaesken . This can overflow and the write_zero function takes size_t anyways. So why do this cast to u4 here?
There was a problem hiding this comment.
It cannot overflow. The array length used is calculated via calculate_array_max_length(), which trims the length so length * type_size is always < 4GB. This is needed, since the packet length is a 32 bit unsigned int in the hprof format.
|
Hello @schmelter-sap, this pull request fulfills all formal requirements. |
| int length = calculate_array_max_length(writer, array, header_size); | ||
| int type_size = type2aelembytes(type); | ||
| // SapMachine 2026-05-06 | ||
| int fill_with_zero = 0; |
There was a problem hiding this comment.
We could move that to the next hunk (after line 1399), so we do not need this lonely comment, right?
|
|
||
| // SAPJVM 2026-05-06: Fill with zeros, if we don't dump the whole content of the array. | ||
| if (fill_with_zero > 0) { | ||
| writer->write_zero((u4) fill_with_zero * type_size); |
There was a problem hiding this comment.
I agree with @MBaesken . This can overflow and the write_zero function takes size_t anyways. So why do this cast to u4 here?
| } | ||
|
|
||
| class ArrayAllocOOMApp extends ArrayAllocApp { | ||
| public static int arraySize = 54321; |
There was a problem hiding this comment.
This is not needed / not used isn't it? The value used by allocArrays() seems to be in ArrayAllocApp.
|
Hello @schmelter-sap, this pull request fulfills all formal requirements. |
|
Hello @schmelter-sap, this pull request fulfills all formal requirements. |
|
Hello @schmelter-sap, this pull request fulfills all formal requirements. |
4 participants
This change adds features to the heap dump for the cf buildpack to be able to store heap dumps for customers in the object store. It includes the following features:
fixes #2250